feat: support PROVES Flight Control Board v5e (SX1262 radio)#18
Open
Mikefly123 wants to merge 5 commits into
Open
feat: support PROVES Flight Control Board v5e (SX1262 radio)#18Mikefly123 wants to merge 5 commits into
Mikefly123 wants to merge 5 commits into
Conversation
Add a v5e board port mirroring the existing v5 port (upstream Pico 2 base + project hardware dtsi) with the v5e's SX1262-based E22-400M30S LoRa module wiring taken from proves-core-reference: busy P13, dio1 P14, TX/RX antenna-switch enables P21/P22, DIO3-controlled 1.8V TCXO (10ms startup), rx-boosted. SPI1 wiring is unchanged from v5. Stop hard-setting CONFIG_LORA_SX127X in prj.conf: both SX12xx drivers default on when their devicetree compatible is present, so the board DT now selects the driver. Hard-setting either one is a fatal unmet- dependency Kconfig warning for boards carrying the other radio. No app changes needed: Main.cpp binds the radio via DT_NODELABEL(lora0), which the v5e port defines. HWIL-verified on a v5e: all 6 one-board main tests pass, including SET_FREQ which performs a live lora_config() on the SX1262. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a PROVES v5e flight controller target with RP2350A/M33 board definitions, SX1262 radio wiring, devicetree-selected LoRa drivers, updated LoRa constants, and documentation for board switching and the required SX126x wake-up patch. Changesv5e board support
Estimated code review effort: 2 (Simple) | ~15 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
⚔️ Resolve merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
The Zephyr SX126x driver polls BUSY immediately after the wake-up SPI transaction, racing the chip's ~340us warm start. When the poll samples BUSY before the chip asserts it, the next SPI command is clocked into a device still starting up and silently ignored. The driver sleeps the radio after every operation and the first command after wake is always SetRfFrequency, so SET_FREQ intermittently failed to retune the SX1262 (7 of 18 register writes dropped in release-build measurements on a v5e; debug builds mask the race via logging delay). Carry the one-line fix (wait out the startup window before polling) as a patch file since the zephyr submodule tracks upstream. Verified on hardware by reading back the RF frequency PLL registers (0x088B) after each write: 0 drops in 35 retunes with the fix, and the two-board RF suite passes in both directions on a mixed GRC(SX1276) + v5e(SX1262) pair. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
jrpear
reviewed
Jul 7, 2026
| + * next command is clocked into a device still starting up and is | ||
| + * silently ignored. Wait out the startup window before polling. | ||
| + */ | ||
| + k_busy_wait(500); |
Collaborator
There was a problem hiding this comment.
I am a bit confused about this patch. We're not sleeping the radio as far as I know. And in any case the datasheet doesn't say "wait after wake" to send commands, it says "wait after sleep".
It is possible the delay just changed the timing of things so as to disguise a race condition causing the intermittent SET_FREQ failures the new README mentions.
…n RX Commit 37b2808 ("Remove unused LoRa payload header", 2026-04-20) zeroed the GRC LoRa payload header to HEADER[0]={}, but the flight firmware's LoRa::receive() always strips sizeof(LoRaConfig::HEADER) bytes from the front of every received LoRa packet, and the flight's LoRaCfg.hpp still defines HEADER[]={0,0,0,0} (4 bytes). Effect: every GRC→LoRa→flight uplink packet arrived with its first 4 bytes removed. Those bytes are the CCSDS TC frame flagsAndScId field (expected value 0x2044). CcsdsTcFrameDetector returned NO_FRAME_DETECTED on every packet; FrameAccumulator's byte-by-byte sliding search never recovered because 0x2044 does not appear anywhere in the TC payload. Commands were silently discarded. The only visible symptom was that lora.m_bytes_received on the flight board kept rising (the LoRa radio layer counts bytes before the frame detector runs) while no command events ever appeared. Fix: restore HEADER[]={0,0,0,0} so the GRC prepends the same 4-byte prefix that the flight expects to strip, keeping the TC frame intact. Verified 2026-06-22 on bench: GET_SEQ_NUM returned EmitSequenceNumber in ~1 s; CMD_NO_OP returned NoOpReceived + OpCodeCompleted in 1.18 s over the full GDS→authenticate-space-data-link→GRC→LoRa→flight path. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ions Both were non-const namespace-scope variables defined in a header included by multiple translation units, creating an ODR violation hazard. Neither is ever modified after initialization: HEADER is only read via sizeof() and memcpy() (which takes const void*), and BANDWIDTH is not referenced via LoRaConfig:: at all (the LoRa component reads bandwidth through F´ params). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ore-lora-header fix(LoRaCfg): restore 4-byte LoRa payload header on v5e branch
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@patches/README.md`:
- Around line 24-27: Label the shell command code block containing the git apply
command with the bash language identifier by changing its opening fence to
```bash.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f57d017d-ae08-4b7b-a0d4-08dbd2ddf734
📒 Files selected for processing (4)
FprimeZephyrReference/project/config/LoRaCfg.hppREADME.mdpatches/README.mdpatches/zephyr-sx126x-wakeup-busy-delay.patch
Comment on lines
+24
to
+27
| ``` | ||
| cd lib/zephyr-workspace/zephyr | ||
| git apply ../../../patches/zephyr-sx126x-wakeup-busy-delay.patch | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Label the shell command block.
Line 24 should use ```bash so Markdown linting can identify the command language.
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 24-24: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@patches/README.md` around lines 24 - 27, Label the shell command code block
containing the git apply command with the bash language identifier by changing
its opening fence to ```bash.
Source: Linters/SAST tools
Collaborator
|
I'm happy to add the new devicetree files but reluctant to accept the Zephyr patch until I understand why we need it |
Collaborator
|
Be advised I have seen the integration tests be a bit flaky, so it's possible that your AI just saw a flaking test and tried random stuff until you got lucky with a successful test. |
Collaborator
|
See #6 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a board port so the GRC firmware runs on the PROVES Flight Control Board v5e in addition to the ground_radio_controller and v5 boards.
boards/proves/proves_flight_control_board_v5e/— mirrors the v5 port (upstream Pico 2 base + project hardware dtsi with dual CDC ACM, fstab, ramdisk). The only substantive difference is the radio: the v5e carries an SX1262-based E22-400M30S module instead of the v5's SX1276. Wiring follows the v5e board definition in proves-core-reference: reset P6, busy P13, dio1 P14, TX/RX antenna-switch enables P21/P22, DIO3-controlled 1.8V TCXO with 10ms startup delay, rx-boosted. SPI1 pins are identical to v5.CONFIG_LORA_SX127X. Both SX12xx drivers aredefault ygated on their devicetree compatible, so the board DT selects the driver. Hard-setting either is a fatal unmet-dependency Kconfig warning for boards carrying the other radio.Main.cppbinds the radio viaDT_NODELABEL(lora0), which the v5e port defines.BOARD=alternative;ground_radio_controllerremains the default.Testing
proves_flight_control_board_v5e/rp2350a/m33; verifiedCONFIG_LORA_SX126X=yand the full sx1262 node in the merged devicetree.SET_FREQat 437.4 MHz, which performs a livelora_config()on the SX1262 and would returnEXECUTION_ERRORon failure.ground_radio_controller— builds green withCONFIG_LORA_SX127X=ystill selected from its devicetree.Notes
LoRaConfig::TX_POWER = 23exceeds the SX1262 driver's +22 dBm limit; worth revisiting before running a v5e at full power (not changed here).bft2) flow flashes one ELF to both boards, so mixed GRC↔v5e RF link testing needs a follow-up workflow change.🤖 Generated with Claude Code
Quick builds for manual testing
For manual testing convenience, the branch
v5e-portable-buildscombines this PR with theportable-buildstooling. Check it out and runmake buildfor a self-bootstrapping build (uv, venv, west workspace, and Zephyr SDK are set up automatically) — no manual environment setup needed. Switch boards by togglingBOARD=insettings.ini, thenmake generatefollowed bymake build. Bothground_radio_controllerandproves_flight_control_board_v5etargets build green on that branch.